fix: copy public directory output instead of input when using Nx #856

lourd · 2021-11-30T08:26:38Z

Summary

When copying the public directory, the build plugin should use the one that Next.js generates as part of the build, not the input one, as there may be files added to the directory as part of the build process. For example, using the InjectManifest plugin from workbox-webpack-plugin like this to produce the specifically named and located file service-worker.js:

// next.config.js
module.exports = {
  webpack: (config, options) => {
    if (!options.dev) {
      const workboxManifestPlugin = new InjectManifest({
        swSrc: "./src/service-worker.ts",
        swDest: "../public/service-worker.js",
      })
      config.plugins.push(workboxManifestPlugin)
    }
  }
}

Test plan

Added a test to the Nx cypress setup. Steps to run:

cd demos/nx-next-monorepo-demo
npm install
npm run build
npx nx run demo-monorepo:serve:production --port=3000

Then in a separate terminal, from the repo root

npm run cy:open

Then click the general.spec.ts. under the nx folder to run it.

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Standard checks:

Check the Deploy Preview's Demo site for your PR's functionality
Add docs when necessary

ascorbic · 2021-11-30T10:56:55Z

src/index.js

@@ -47,14 +47,14 @@ module.exports = {

    checkNextSiteHasBuilt({ publish, failBuild })

-    const { appDir, basePath, i18n, images, target, ignore } = await getNextConfig({ publish, failBuild })
+    const { appDir, basePath, i18n, images, target, ignore, outdir } = await getNextConfig({ publish, failBuild })


Can you explain where you're getting this config name? I'm not aware of outdir being a standard config value.

It's added when you're using Next.js with Nx. You can see the source code adding it here

ascorbic · 2021-11-30T10:59:31Z

The public dir is used as-is, not generated by Next. It's just meant to be used for static files. If you want to generate other files you should be creating them in .next. Are you referring to somethign else?

lourd · 2021-11-30T15:56:22Z

Ah, looks like this is a difference when using Next with Nx. Nx copies the public directory over to the specified output directory, where it also puts the .next directory. I just tried a project started with create-next-app and it doesn't have config.outdir in required-server-files.json, and it doesn't create a new public directory, even when specifying an alternative distDir.

The issue with putting generated files in the generated static directory is that they're then served from /static or /_next/static. Some files need to served from root — for example, the /.well-known/apple-app-site-association that makes deep linking work on iOS. And some of those files get modified or created at build time. What's a good way to go about that with vanilla Next.js, without Nx?

Would you support updating the PR to conditionally use/join outdir to appDir if it exists, to support this Nx-specific case?

lourd · 2021-11-30T17:53:40Z

I went ahead and made it conditional with a comment explaining the case. I also made a Nx workspace with a Next.js project to demonstrate -- https://github.com/lourd/nx-next-public-example. To see what I'm talking about, clone it and run yarn build. It will generate a directory at dist/apps/next-public-test which as the .next directory, a public directory, and next.config.js. If you look in the required-server-files.json you'll see the outdir property with a value of "../../dist/apps/next-public-test". And the public directory has the files that were in public to start with (star.svg, nx-log-white.svg) as well as the service-worker.js file that was specified added by the InjectManifest plugin as specified in the webpack setup in next.config.js.

If you want, you can also run the build to see the service worker getting registered and loaded from that public location. The command for that is yarn nx run next-public-test:serve:production. You'll only see the service worker logs the first time the page loads — to see it on subsequent loads you need to unregister it through the browser's dev tools.

lourd · 2021-11-30T20:17:18Z

I also just added a test to the Cypress suite for nx-next-monorepo-demo. Updated the test plan in the PR description with steps to run it. Please let me know if there's anything else I can do!

netlify · 2021-12-01T23:29:23Z

‼️ Deploy request for netlify-plugin-nextjs-static-root-demo rejected.
Learn more about Netlify's sensitive variable policy

🔨 Explore the source changes: 49c5147

tiffafoo · 2021-12-03T11:24:48Z

cypress/integration/nx/general.spec.ts

+
+  it('serves generated public files', () => {
+    cy.request(`service-worker.js`)


You can be more explicit with the assetion here by making sure the status returned is a 200. Something like this:

Suggested change

cy.request(`service-worker.js`)

cy.request('service-worker.js').then((res) => {

expect(res.status).to.eq(200)

})

👍 went ahead and added that and a check for the content-type (in case it's serving some fallback html instead of js!)

tiffafoo

Thank you for the test steps, comments and taking the time to explain the nx use case, super appreciated! I tested using your test steps with and without the outdir as well as using netlify deploy --build and things seem to be behaving as expected 🎉 .

lourd · 2021-12-03T14:35:10Z

😃 For sure! Please let me know if there's anything else this needs to be merged.

kodiakhq · 2021-12-06T09:38:57Z

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

opennextjs deleted a comment from netlify bot Nov 30, 2021

tiffafoo added the type: bug code to address defects in shipped code label Nov 30, 2021

ascorbic reviewed Nov 30, 2021

View reviewed changes

tiffafoo added the status: needs reproduction this issue needs to provide a repo that reproduces the bug described label Nov 30, 2021

lourd changed the title ~~fix: copy public directory output instead of input~~ fix: copy public directory output instead of input when using Nx Nov 30, 2021

lourd requested a review from ascorbic November 30, 2021 20:20

tiffafoo removed the status: needs reproduction this issue needs to provide a repo that reproduces the bug described label Dec 1, 2021

tiffafoo assigned ascorbic Dec 1, 2021

ascorbic assigned tiffafoo and unassigned ascorbic Dec 3, 2021

tiffafoo reviewed Dec 3, 2021

View reviewed changes

tiffafoo previously approved these changes Dec 3, 2021

View reviewed changes

lourd dismissed tiffafoo’s stale review via 49c5147 December 3, 2021 14:28

ascorbic added the automerge label Dec 3, 2021

Louis DeScioli added 2 commits December 3, 2021 14:43

fix: copy public directory from outdir when exists

1e6f921

chore: add service-worker test to nx cypress

a77d788

kodiakhq bot removed the automerge label Dec 6, 2021

Merge branch 'main' into fix/copy-public-output

dbddf12

ascorbic added the automerge label Dec 6, 2021

tiffafoo approved these changes Dec 6, 2021

View reviewed changes

kodiakhq bot merged commit d959f82 into opennextjs:main Dec 6, 2021

lourd mentioned this pull request Aug 29, 2022

Copy public directory output instead of input when using Next.js with Nx vercel/vercel#8482

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: copy public directory output instead of input when using Nx #856

fix: copy public directory output instead of input when using Nx #856

Uh oh!

lourd commented Nov 30, 2021 •

edited

Loading

Uh oh!

ascorbic Nov 30, 2021

Uh oh!

lourd Dec 2, 2021

Uh oh!

ascorbic commented Nov 30, 2021

Uh oh!

lourd commented Nov 30, 2021 •

edited

Loading

Uh oh!

lourd commented Nov 30, 2021

Uh oh!

lourd commented Nov 30, 2021

Uh oh!

netlify bot commented Dec 1, 2021 •

edited

Loading

Uh oh!

tiffafoo Dec 3, 2021

Uh oh!

lourd Dec 3, 2021

Uh oh!

tiffafoo left a comment •

edited

Loading

Uh oh!

lourd commented Dec 3, 2021

Uh oh!

kodiakhq bot commented Dec 6, 2021

Uh oh!

Uh oh!


		it('serves generated public files', () => {
		cy.request(`service-worker.js`)

-    cy.request(`service-worker.js`)
+    cy.request('service-worker.js').then((res) => {
+      expect(res.status).to.eq(200)
+    })

fix: copy public directory output instead of input when using Nx #856

fix: copy public directory output instead of input when using Nx #856

Uh oh!

Conversation

lourd commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Standard checks:

Uh oh!

ascorbic Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

lourd Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

ascorbic commented Nov 30, 2021

Uh oh!

lourd commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lourd commented Nov 30, 2021

Uh oh!

lourd commented Nov 30, 2021

Uh oh!

netlify bot commented Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiffafoo Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

lourd Dec 3, 2021

Choose a reason for hiding this comment

Uh oh!

tiffafoo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lourd commented Dec 3, 2021

Uh oh!

kodiakhq bot commented Dec 6, 2021

Uh oh!

Uh oh!

lourd commented Nov 30, 2021 •

edited

Loading

lourd commented Nov 30, 2021 •

edited

Loading

netlify bot commented Dec 1, 2021 •

edited

Loading

tiffafoo left a comment •

edited

Loading